Derive the loopback listener inventory from the lint - #635
Conversation
…opback inventory scripts/direct-interop/run.mjs binds a loopback HTTP server at :214 and has since it landed, so loopback-lint reports four listeners while the spec's "Loopback Listeners" section still says "Today the set is three". The harness is guarded — it imports isOwnOrigin from lib/src/host/loopback-guard.ts and isAuthorized from standalone/scripts/dev-host-guard.mjs — so the lint is green and only the spec's own inventory is stale. Nothing else changes: the rule, the FAIL IF, and the lint are all untouched. Found by the 2026-09-13 nightly security audit (W1, refs #598).
Deploying mouseterm with
|
| Latest commit: |
973b70e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0c8d2e16.mouseterm.pages.dev |
| Branch Preview URL: | https://fix-loopback-inventory-fourt.mouseterm.pages.dev |
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
Feedback on work in progress, not a merge verdict — mark the PR ready when you want the full review.
The correction itself checks out: scripts/direct-interop/run.mjs binds server.listen(0, '127.0.0.1', …), imports both isOwnOrigin and isAuthorized, and gates on them at the top of the handler, so it passes the lint by reference rather than by allowlist. loopback-lint reports 4 loopback listeners, 1 allowlisted and its self-test stays green at 7 load-bearing checks; the two spec-lint path misses are the standalone/sidecar/node_modules artifacts of an un-installed checkout, not this diff.
One thing keeps the fix from holding: the sentence's derivation instruction still says "shipped trees", which is a narrower scope than the four it now counts, and .github/audit/application-security.md repeats that instruction to the very domain that raised W1. Inline suggestion below.
One case the wider scope surfaces and this bullet doesn't address either way: relay/scripts/dev.mjs defaults DORMOUSE_BIND_HOST to 127.0.0.1 and calls server.listen(port, env.DORMOUSE_BIND_HOST), a loopback HTTP-and-WebSocket bind the regex can't see because the host is a runtime value — the case the lint's header names as its own ceiling. The front matter defers the network boundary to docs/specs/security-remote.md, so it may well be out of scope here, but nothing in the bullet says so, and an auditor deriving mechanically lands on it. Saying once why the relay is not in this set would stop the next derivation re-opening it.
…e set it names The bullet said to derive the set by searching the "shipped trees", but two of the four listeners it now names — the browser-dev bridge and the direct-path interop harness — are in neither shipped tree; loopback-lint reaches both only because it scans git ls-files for tracked, non-test source files. An auditor following the old instruction derives two and re-files the same miscount. .github/audit/application-security.md repeated the instruction verbatim to the domain that raised it, so both lines move together. Also state once why the Relay is out of this set. It binds from config rather than to render a surface, and the front matter already defers the network boundary to docs/specs/security-remote.md — but nothing in the bullet said so, and the wider derivation scope now lands a mechanical search on relay/scripts/dev.mjs. docs/specs/security-local.md: 2533 / 2550 words, no re-baseline.
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
The count, the derivation instruction, and the audit prompt all check out: pnpm lint:loopback reports 4 loopback listeners, 1 allowlisted, scripts/direct-interop/run.mjs gates on isAuthorized and isOwnOrigin before routing, and sourceFiles() in scripts/loopback-lint.mjs really does scan git ls-files for tracked, non-test source files — so .github/audit/application-security.md:45 now names the scope the lint scans.
Two things the correction leaves behind.
The Relay deferral doesn't close the question it was added to close. Inline suggestion on the bullet — the stated reason isn't true of the one Relay bind that is loopback, and the spec-level pointer lands the reader on the section about the installer rather than the one about what the listener admits.
scripts/loopback-lint.mjs's own header still narrates the pre-PR count. Its "Why this exists" paragraph says "Two of the three listeners we ship got that wrong at some point" and "so a fourth listener fails a build instead of waiting for the next audit to notice it" — written when the set was three, and now read by an editor at the moment they are adding a bind form. That is the same count-shaped claim nothing pins that this PR exists to correct, one file over, and "a fourth listener" now names one that already exists. I'm pushing a count-free rewording of those two clauses rather than incrementing them, so the next listener doesn't reopen this.
…nswers it The bullet said the Relay was out of scope because "it binds from config", but relay/scripts/dev.mjs defaults DORMOUSE_BIND_HOST to 127.0.0.1 — the one Relay bind a mechanical derivation lands on is loopback, so the stated reason did not cover it. Name the dev runner, and point at security-remote.md -> "Cross-origin access" (no CORS, no cookie auth) rather than the spec as a whole, which sends the reader to "Network posture" and the installer's bind pin instead. Word-neutral: 2649 / 2650 merged with main. Drop the matching count from scripts/loopback-lint.mjs's header, which still narrated three listeners and a hypothetical fourth. The inventory lives in the spec; a count in two places drifts in one of them.
ca39cc7 to
e754820
Compare
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
The count, the derivation scope, and the audit prompt all hold: pnpm lint:loopback reports 4 loopback listeners, 1 allowlisted, scripts/direct-interop/run.mjs gates on isAuthorized and isOwnOrigin before routing, and loopback-lint-selftest stays green at 7 load-bearing checks. The lint-header rewording lands — no inventory count survives in that file.
The Relay exclusion is now false in the other direction. The bullet says "only relay/scripts/dev.mjs binds loopback", but relay/src/start.ts binds serve({ fetch, port, ...(bindHost ? { hostname: bindHost } : {}) }) with bindHost from DORMOUSE_BIND_HOST, and docs/specs/security-remote.md -> "Network posture (self-hosted)" pins that to 127.0.0.1 in all three installers — "The shipped self-host deployment is a per-login user agent bound to loopback". So the shipped Relay binds loopback, and an auditor who checks the pointer finds the exclusion contradicted by the spec it cites. scripts/loopback-lint.mjs's own header states the accurate version one file over: "relay/ does it deliberately from config". The reason that does hold is the one the pointer already names — no foreign browser origin may drive the API, whatever interface it binds. Inline suggestion below.
The fix needs budget headroom this branch doesn't have. The branch is behind main on this spec: docs/specs/security-local.md is 2533 words at e754820b but 2649 on the merge ref, against main's 2650 budget — one word. The suggestion is +4, and ratcheting on the branch as it stands would write 2550 into scripts/spec-word-budgets.json, which the merge would then prefer over main's 2650 and fail at 2653. Merging main in first, then ratcheting, is what keeps CI green.
One thing the diff leaves adjacent: .github/audit/application-security.md still opens that paragraph with "read lib/src/host/loopback-guard.ts first — it states the rule — then each listener it names", and the guard names no inventory on purpose — its header says the spec "is deliberately not restated here, since it tells its reader to derive that set by search rather than trust a list". An auditor reading "each listener it names" finds one mention of the iframe proxy and no set. Since the next sentence in the same paragraph is what this PR corrects, the antecedent belongs to the section, not the guard.
Pushing all of it — the merge, the reworded exclusion, the ratchet, and the antecedent — as one commit.
…re it binds `relay/src/start.ts` binds `hostname: bindHost` from `DORMOUSE_BIND_HOST`, and every installer pins that to 127.0.0.1 per `docs/specs/security-remote.md` -> "Network posture (self-hosted)" — so "only relay/scripts/dev.mjs binds loopback" was falsified by the very section the bullet points at. Rest the exclusion on what the listener admits instead: no foreign browser origin may drive the API, whatever interface it binds. That survives any bind host and matches the "Cross-origin access" heading the pointer already names. Also fix the antecedent in the audit prompt: `loopback-guard.ts` deliberately names no inventory, so "each listener it names" sends the auditor to a file that has none. The section is what names them. Merges main first so the ratchet is computed against the size CI sees: the spec lands at 2657, so the budget goes to 2700. (The review's "+4 words → 2653" predated the ", whatever interface it binds" clause; the ratchet is needed either way.)
nedtwigg
changed the title
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
The lint change holds where it counts: node scripts/loopback-lint.mjs on the
merge ref prints four non-test listeners, seven test listeners and nine
self-test fixtures, and loopback-lint-selftest stays green at 8 load-bearing
checks — so the new test-classification branch is pinned the same way the seven
bind forms are. spec-lint is clean apart from the two
standalone/sidecar/node_modules path misses an un-installed checkout always
reports.
The word budget is 50 words looser than the file earns. countWords puts
docs/specs/security-local.md at 2605 on the merge ref, which rounds to 2650 —
the value main already carries. The 2657 → 2700 arithmetic in 1ec1323b's
message was true of that commit, but da7e6a0f rewrote the bullet shorter and
the ratchet was not re-run. AGENTS.md pins a budget to "its size rounded up to
the nearest 50", and check 10 only fails upward, so a stale-high budget rides
green through CI while handing the next editor 95 unearned words — two rules at
the 46-word corpus median the step size exists to make expensive. The whole
scripts/spec-word-budgets.json hunk can come out of the PR.
Three smaller ones inline: the self-test's failure hint quotes a spec clause
this PR deleted, the audit prompt routes the auditor through the mutating
self-test, and withAppendedOutput re-implements two things lint-kit already
has.
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
The inventory move holds where it counts: node scripts/loopback-lint.mjs prints
four non-test listeners, seven test listeners and nine self-test fixtures,
loopback-lint-selftest stays green at 8 load-bearing checks, and
scripts/spec-word-budgets.json is out of the diff — docs/specs/security-local.md
is inside main's 2650 without a ratchet. The self-test hint now quotes a clause
that exists (A new non-test listener without a guard reference fails the build,
docs/specs/security-local.md line 119), and node scripts/loopback-lint.mjs in
the audit prompt gets the auditor the inventory without the self-test's eight file
mutations.
One gap the new wording opens.
The lint's file filter is narrower than the scope three files now claim for it.
SOURCE_EXT in scripts/loopback-lint.mjs is /\.(?:ts|tsx|js|jsx|mjs|cjs)$/ —
every JavaScript variant, but only two of TypeScript's four. The spec bullet now
says the lint "scans all tracked JavaScript and TypeScript and prints every bind it
recognizes", .github/audit/application-security.md says "The lint scans all
tracked JavaScript and TypeScript" and then sends the auditor to "the same files"
for the manual sweep, and the lint's own header says "scans every tracked
JavaScript and TypeScript file". A loopback bind in a .mts or .cts file is
caught by none of the three: pnpm test stays green with no guard reference, the
printed inventory omits it, and the nightly auditor reading "the same files"
excludes it too. vscode-ext/vitest.config.mts and
vscode-ext/vitest.smoketest.config.mts are tracked today, so the extension is in
use — it is the count-shaped drift this PR removes from the spec, relocated one
extension over.
Neither .mts file binds anything, so widening the filter changes today's output
not at all. Inline suggestion below; pushing it as a commit, since it is the fix
this review already worked out and the PR has no human author to apply it.
The spec bullet, the application-security prompt and this lint's own header all say it scans every tracked JavaScript and TypeScript file, but SOURCE_EXT listed every JavaScript variant and only two of TypeScript's four. Two tracked .mts files exist, so a loopback bind in one was invisible to the deterministic check and excluded from the manual sweep the prompt sends the auditor on. The self-test gains a case that appends a bind to a tracked .mts file and requires the lint to go red, so the extension scope is pinned the same way the seven bind forms and the test classification are. Verified load-bearing: narrowing SOURCE_EXT back makes the new case the only weak one.
2 participants
The security spec had a hand-maintained listener count and list. They drifted when scripts/direct-interop/run.mjs added another loopback listener.
This PR removes that duplicated inventory. The lint is now the live inventory:
The audit prompt tells the auditor to inspect every non-test result, skip the clearly labeled test and self-test results, and search the same files manually for new APIs and hosts built at runtime.
The spec no longer contains a count or path list that can drift. It keeps the rule and the lint's known ceiling. The Relay remains separate because no foreign browser origin may drive its API, whatever interface it binds; security-remote.md owns that rule.
Why not say all source code? The lint only scans tracked JavaScript and TypeScript. Saying all source code would also claim Rust, shell, generated files, dependencies, and server APIs the matcher does not know.
Current live inventory: four non-test bind sites, seven test bind sites in six files, and nine textual matches in the self-test fixture. These numbers are output, not copied into the spec.
Verification: pnpm test passes on the rebased branch.